Skip to content

fix(tern): settle grouped applies when the engine loses in-flight work - #1192

Open
aparajon wants to merge 4 commits into
armand/progress-display-transition-splitfrom
armand/grouped-lost-work
Open

fix(tern): settle grouped applies when the engine loses in-flight work#1192
aparajon wants to merge 4 commits into
armand/progress-display-transition-splitfrom
armand/grouped-lost-work

Conversation

@aparajon

@aparajon aparajon commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #1208, which separates a progress poll's display refresh from the state it claims. Review that first; this PR's diff is against it.

Why this matters

Follow-up to #1113, which settled lost engine work for the sequential drive only. The grouped drive — all Vitess applies, and MySQL under defer_cutover — had no equivalent: an engine that lost its in-flight work kept answering progress polls with "no active schema change", which maps to pending, so the drive polled forever and the apply never left running. It held the database's active-apply slot for as long as the process lived, queueing every later change to that database behind work that no longer existed.

BEFORE — the engine forgets; the drive never finds out

    ┌──────────────────────────────────────────────┐
    │                                              │
    ▼                                              │
  poll ──▶ engine: "no active schema change"       │
           storage: tasks still in flight          │
                     │                             │
                     └── maps to pending ──────────┘
                         nothing settles

  the apply never leaves running
  the database's active-apply slot is never released      ✗

What it does

An engine reports "no active schema change" for two very different reasons: it is still setting the work up, or the work is gone. The report alone does not distinguish them — but waiting a fixed period does, because a healthy engine starts reporting real progress and one that has lost the work never will. Past that period engine progress can never terminalize the tasks, so the live target schema is the only remaining authority and the drive reads it.

AFTER — wait a fixed period, then go ask the target

  poll ──▶ engine: "no active schema change"
                     │
         ┌───────────┴────────────┐
         │                        │
  no task in flight         task in flight
         │                        │
         ▼                        ▼
  nothing is wrong;      how long has the engine
  normal tick            been saying this?
                                  │
                    ┌─────────────┴─────────────┐
                    │                           │
           under the grace period      past the grace period
                    │                           │
                    ▼                           ▼
           keep polling — the           stop believing the engine;
           engine may be restarting     read the live target schema
           or still provisioning        once, and settle each
                                        in-flight task from it:

                                          revert phase   ─▶ retryable
                                          already landed ─▶ completed
                                          still needed   ─▶ retryable
                                          unreadable     ─▶ counts as an error
                                                                │
                                                   enough errors in a row
                                                                ▼
                                                         apply retryable,
                                                         freed for a fresh
                                                         claim to re-drive   ✓
  • Detection at the apply level. A successful poll that maps to pending while any stored task is in flight starts the grace period — the same engine-declared budget the sequential drive uses, and zero for engines that register accepted work before returning, since those have no setup phase to wait out. One timer covers the whole drive: grouped work is a single engine operation, so the engine loses or keeps all of it together.
  • Settlement per task. Every in-flight task settles from one shared re-plan of the reviewed schema set: completed when its (namespace, shard, table) change already landed, retryable when the target still needs it. Tasks already terminal or at rest are never re-settled.
  • Settlement is authoritative on its own. Settlement names the tasks it resolved and the same tick's progress projection leaves them out — the poll that sent the drive to the target reports no active schema change, so it carries neither progress to display nor a state those tasks may take. Nothing depends on a downstream guard rejecting a claim the poll should never have made.

Two safety properties worth calling out:

  • A schema read can never settle a revert. Once the forward change has cut over the live schema matches the reviewed target whether or not the revert ran, so a converged target says nothing about a revert's outcome. Revert-phase tasks always rest retryable without a target read, and settle even when the re-plan fails — completing them would report a successful schema change while the change being undone is still in place.
  • Verification failures are bounded and never terminal. A failed plan load or re-plan counts against the same consecutive-error budget as a failed progress poll; exhausting it pauses the apply retryable for a fresh claim, never permanently failed, because nothing proved the target is broken.

The settled task states drive the existing aggregate derivation in the same tick, so the apply quiesces — or the operation drive exits for operator projection — through the normal paths. The per-task settlement and re-plan helpers are shared with the sequential drive rather than duplicated.

Opened by Claude (Fable 5); summary revised by Claude (Opus 5).

Copilot AI lite review requested due to automatic review settings August 28, 2026 09:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the “lost engine in-flight work” settlement mechanism (introduced for the sequential drive) to the grouped/atomic drive path (Vitess applies and MySQL defer_cutover), preventing applies from polling pending forever when the engine forgets accepted work and continuing to hold the database’s active-apply slot.

Changes:

  • Add grouped-drive detection of “engine reports pending/no active work while durable tasks are in-flight” and apply a bounded trust budget before settling from the target schema.
  • Settle grouped in-flight tasks using a single shared re-plan of the reviewed schema set; revert-phase tasks are always settled retryable without reading the target.
  • Add focused grouped-drive progress tests covering converged targets, non-converged targets, bounded verification failures, self-healing stale snapshots, and revert-phase behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/tern/local_control_resume.go Extracts a reusable helper to re-plan the reviewed schema set against the live target and index remaining changes by (namespace, shard, table).
pkg/tern/local_apply_sequential.go Refactors lost-work settlement into clearer helpers reused by grouped settlement; adds lost-work tracking state for shared polling structures.
pkg/tern/local_apply_grouped.go Implements grouped/atomic lost-work detection with a trust budget and per-task settlement driven by a shared target re-plan.
pkg/tern/local_apply_grouped_progress_test.go Adds test coverage for grouped lost-work detection and settlement outcomes, including revert-phase handling and bounded verification errors.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/tern/local_apply_grouped_progress_test.go
@aparajon
aparajon marked this pull request as ready for review August 29, 2026 16:29
aparajon and others added 4 commits August 30, 2026 11:25
The grouped drive now detects the same engine-vs-storage divergence the
sequential drive settles: a progress poll reporting no active schema change
while stored tasks are in flight. One apply-level tracker spends the
engine-declared trust budget (zero for engines with synchronous work
registration), then each in-flight task settles from one shared re-plan of
the reviewed schema set — completed when its change already landed,
retryable when the target still needs it, and revert-phase tasks always
retryable because a schema read cannot settle a revert. Verification
failures count against the same bounded consecutive-error budget as failed
polls, pausing the apply retryable when exhausted instead of polling
forever and holding the database's active-apply slot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…it holds

The table field carried an engine task-state string but was named for
engine.State, which reads as a different type than the one the helper
takes. Name it after the parameter it feeds.
…ified

A revert-phase task settles without reading the target, so an unreadable
plan must not strand it alongside the tasks that do need verifying. The
mixed case was documented but unproven: every existing case had either
all revert-phase tasks or a readable plan, so nothing failed if the
revert settlement moved behind the plan load.
Once the live target schema has answered for a task, the poll that sent the
drive there reports no active schema change: it carries neither progress to
display nor a state the task may take. Settlement now names the tasks it
resolved and the progress projection leaves them out, instead of writing them
again and leaning on the no-backward guard downstream to reject the claim.
@aparajon
aparajon force-pushed the armand/grouped-lost-work branch from 0357c2b to 25cb3d6 Compare August 30, 2026 03:33
@aparajon
aparajon changed the base branch from main to armand/progress-display-transition-split August 30, 2026 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants